Skip to content

Defer MixpanelFlutterPlugin registration and initialization to prevent ANRs #191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
May 29, 2025

Conversation

jaredmixpanel
Copy link
Collaborator

@jaredmixpanel jaredmixpanel commented May 22, 2025

Defer MethodChannel setup from onAttachedToEngine into handleInitialize(...), so the channel is only created when Dart calls initialize(). This lazy registration prevents blocking the main thread during engine startup (including headless/Firebase isolates) and eliminates the ANR.

Key Changes
• Move new MethodChannel(...) + setMethodCallHandler(this) into handleInitialize() guarded by channel == null && flutterPluginBinding != null
• Leave onAttachedToEngine() free of any codec or messenger work
• Ensure flutterPluginBinding is stored on attach for later use in initialization

@jaredmixpanel jaredmixpanel requested a review from Copilot May 22, 2025 23:22
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR defers the registration and initialization of the MixpanelFlutterPlugin to prevent potential ANRs during plugin registration. It implements lazy initialization for the MethodChannel, stores the FlutterPluginBinding for later use, and ensures proper cleanup in onDetachedFromEngine.

  • Lazy initialization of the MethodChannel in handleInitialize
  • Storing the FlutterPluginBinding for deferred channel registration
  • Cleaning up channel and binding references in onDetachedFromEngine

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
android/src/main/java/com/mixpanel/mixpanel_flutter/MixpanelFlutterPlugin.java Defers channel registration and adds lazy initialization to avoid ANRs
README.md Updates Markdown formatting for consistency in the “I want to know more!” section

@jaredmixpanel jaredmixpanel marked this pull request as ready for review May 22, 2025 23:39
@jaredmixpanel jaredmixpanel requested a review from Copilot May 29, 2025 23:12
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR defers the creation and registration of the MethodChannel from plugin attachment to the explicit Dart initialize() call, preventing main-thread blocking and ANRs during engine startup.

  • Lazily initialize the Flutter MethodChannel in handleInitialize() using stored flutterPluginBinding
  • Store flutterPluginBinding and context in onAttachedToEngine() only, and clean up in onDetachedFromEngine()
  • Updated helper code with Java diamond operator and cleaned up README formatting

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
android/src/main/java/com/mixpanel/mixpanel_flutter/MixpanelFlutterPlugin.java Moved channel setup into handleInitialize(), stored binding/context on attach, and cleared them on detach
android/src/main/java/com/mixpanel/mixpanel_flutter/MixpanelFlutterHelper.java Added generic diamond operator (new ArrayList<>())
README.md Removed extra bullets, fixed TOC formatting, and added DeepWiki badge
Comments suppressed due to low confidence (2)

android/src/main/java/com/mixpanel/mixpanel_flutter/MixpanelFlutterPlugin.java:185

  • Consider adding unit or integration tests to verify that the MethodChannel is initialized lazily in handleInitialize() and that it’s cleaned up properly in onDetachedFromEngine().
if (channel == null && flutterPluginBinding != null) {

README.md:103

  • Use 'GitHub' instead of 'Github' to match proper branding.
star or watch our repository on [Github](https://github.com/mixpanel/mixpanel-flutter).

@jaredmixpanel jaredmixpanel added the bug Something isn't working label May 29, 2025
@jaredmixpanel jaredmixpanel requested a review from Copilot May 29, 2025 23:35
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR defers the Flutter MethodChannel registration and initialization for Mixpanel plugins to avoid ANRs by moving channel setup into the handleInitialize method (lazy initialization). It also updates the web interop helper, cleans up plugin teardown, and refreshes documentation.

  • Moved MethodChannel creation from onAttachedToEngine to a new initializeMethodChannel() called in handleInitialize.
  • Preserved flutterPluginBinding and context on attach and cleared them on detach.
  • Extended and refined the safeJsify helper for Dart-to-JS conversions and adjusted web API calls; updated README content.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

File Description
lib/mixpanel_flutter_web.dart Expanded safeJsify logic & adjusted JS calls
android/src/.../MixpanelFlutterPlugin.java Added lazy init, initializeMethodChannel(), cleanup
android/src/.../MixpanelFlutterHelper.java Added generics (ArrayList<>) for type safety
README.md Updated Table of Contents, fixed GitHub link & layout
Comments suppressed due to low confidence (4)

lib/mixpanel_flutter_web.dart:43

  • The fallback branch returns raw Dart values for unsupported types, which may lead to JS interop type mismatches. Consider throwing for unsupported types or explicitly handling all expected input types.
JSAny? safeJsify(dynamic value) {

README.md:9

  • The Table of Contents only lists two sections but other sections (e.g., Check for Success, I want to know more) were added later. Please update the TOC to reflect all headings.
- [Introduction](#introduction)

android/src/main/java/com/mixpanel/mixpanel_flutter/MixpanelFlutterPlugin.java:193

  • Lazy initialization was introduced for the MethodChannel. Consider adding unit tests to verify initializeMethodChannel() is only invoked when handleInitialize runs and not during onAttachedToEngine.
initializeMethodChannel();

lib/mixpanel_flutter_web.dart:412

  • [nitpick] Using safeJsify(<dynamic>[]) as JSArray for an empty array is more verbose and risks a null cast. You could revert to <JSAny>[].jsify() for clarity and direct creation of a JSArray.
.union(name, value is JSArray ? value : safeJsify(<dynamic>[]) as JSArray);

@jaredmixpanel jaredmixpanel requested a review from Copilot May 29, 2025 23:47
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR defers the registration and initialization of the MethodChannel to prevent ANRs during engine startup by enabling lazy initialization.

  • Moves MethodChannel creation and handler setup from onAttachedToEngine to handleInitialize
  • Stores flutterPluginBinding for later use in initialization
  • Updates web interop conversion in safeJsify and adjusts README content

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

File Description
lib/mixpanel_flutter_web.dart Adds safeJsify utility with extended type handling and updates its usage in init and people_track_charge calls
android/src/main/java/com/mixpanel/mixpanel_flutter/MixpanelFlutterPlugin.java Defers MethodChannel initialization via lazy initialization and stores plugin binding for later use
android/src/main/java/com/mixpanel/mixpanel_flutter/MixpanelFlutterHelper.java Updates ArrayList initialization syntax
README.md Improves Markdown formatting and updates GitHub link reference
Comments suppressed due to low confidence (1)

lib/mixpanel_flutter_web.dart:61

  • [nitpick] Consider using a dedicated logging library or Flutter's debugPrint instead of print for production logging to better manage log outputs and log levels.
print('[Mixpanel] Warning: Unsupported type for JS conversion: ${value.runtimeType}. ' + 'Value will be ignored. Supported types are: Map, List, DateTime, bool, num, String, JSAny, and null.');

@jaredmixpanel jaredmixpanel merged commit e850b9a into main May 29, 2025
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant